[Security Solution][Detections] Implement writing rule execution events to event_log #112286

xcrzx · 2021-09-15T14:49:56Z

Addresses: #106469

Summary

Refactored logExecutionMetrics following our decision to log multiple metrics in a single document.
Extended Event Log schema with fields required for Rule Execution log: status, metrics, etc. (see the RFC)
Removed deprecated RuleRegistry based implementation
Implemented write methods, all reads return empty data structures for now
Replaced the hardcoded feature switch with an experimental config flag.

To enable EventLog-based client:

echo "xpack.securitySolution.ruleExecutionLog.underlyingClient: eventLog" >> ./config/kibana.dev.yml

Checklist

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2021-09-28T13:29:23Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

elasticmachine · 2021-09-28T13:29:25Z

Pinging @elastic/security-solution (Team: SecuritySolution)

spong

Checked out, tested locally, and reviewed code. Clean implementation and easy to follow, thank you for adding the appropriate deprecations as well. 🙂

Testing was verified against the existing rules (w/o the experimental ruleRegistryEnabled flag), and confirmed event-log documents correctly included the following new fields:

kibana.alert.rule.execution.status
kibana.alert.rule.execution.status_order
kibana.alert.rule.execution.metrics.execution_gap_duration_s
kibana.alert.rule.execution.metrics.indexing_duration_sum_ms
kibana.alert.rule.execution.metrics.search_duration_sum_ms

Did notice a few 0 entries for kibana.alert.rule.execution.metrics.indexing_duration_sum_ms, but doesn't look to be introduced as part of this PR since you're just writing the existing result.bulkCreateTimes.

Also added a nit about including ignore_above in the mapping, and updating the RFC's provider format, but all else LGTM! Thanks @xcrzx! 👍 🙂 🚀

x-pack/plugins/event_log/generated/mappings.json

...urity_solution/server/lib/detection_engine/rule_execution_log/event_log_adapter/constants.ts

banderror

Thank you @xcrzx, this LGTM 👍 I left a few comments.

x-pack/plugins/event_log/generated/mappings.json

...olution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_client.ts

...lution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_adapter.ts

...olution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_client.ts

pmuellr

Left a bunch of questions / comments. Overall looking good! But there are two kinda blockers:

left a comment showing an existing alerting event doc, with some suggestions for existing fields used by alerting that should probably be added here, if possible
no function tests :-). We have some you can probably copy the style from - or at least steal some of our utilities to wait for events to arrive which are pretty useful - the getEventLog() function used in the linked test module.

x-pack/plugins/event_log/generated/mappings.json

...lution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_adapter.ts

...olution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_client.ts

...urity_solution/server/lib/detection_engine/rule_execution_log/event_log_adapter/constants.ts

x-pack/plugins/event_log/generated/mappings.json

xcrzx · 2021-10-05T12:42:08Z

I've extended the existing event log schema test with the new fields. Not sure if there are more tests needed as I've touched only schema. On the solution side, we decided not to add tests right now as the exec log is under active development and hidden under a feature flag.

@pmuellr, could you please take a look at this PR one more time? We still have an active discussion in the RFC regarding field names, but if that's okay with you, I can make a follow-up PR with field name changes once we have agreed upon them. Exec log implementation is hidden under a feature flag, so only developers will use it; therefore, it should be fine to merge the current field names and change them later. Meantime this PR will allow us to start implementing other Exec Log methods and refactor existing API routes.

banderror

I submitted a few suggestions, but I'm ok with merging this PR right away and addressing the rest of the field-related considerations in a follow-up PR. We need this code in Security to unblock the next tasks.

@xcrzx 💪 🙌

...olution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_client.ts

pmuellr

LGTM; ran Kibana, added an index threshold rule and got it firing, took a peek at the event log from Kibana, everything looks good.

Left a few notes ...

pmuellr · 2021-10-07T20:15:26Z

...olution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_client.ts

+      event: {
+        kind: 'metric',
+        action: RuleExecutionLogAction['execution-metrics'],
+        sequence: this.sequence++,


Since sequence isn't persisted, it will "roll over" to 0 when Kibana is restarted, and if there are multiple Kibana instances, they will be writing the same sequence values out while they are running, starting at 0.

Note that we do have field kibana.server_uuid which uniquely identifies each Kibana instance in case there are multiple.

I assume that's ok, but thought I'd point it out.

I think it should be fine. We will use sequence as the second sorting argument (sort: ['@timestamp', 'sequence']). I.e., we need sequence only to sort events logged in a batch with the same @timestamp value. So we don't need to handle Kibana restarts and distinguish between multiple instances.

pmuellr · 2021-10-07T20:25:35Z

x-pack/test/plugin_api_integration/test_suites/event_log/service_api_integration.ts

@@ -150,6 +150,21 @@ export default function ({ getService }: FtrProviderContext) {
        },
        kibana: {
          saved_objects: [savedObject],
+          space_ids: ['space id'],


This test is sufficient for the event log portion of the PR - it ensures that we can actually generate and then read back the same data.

But I assumed you'd want to add some tests for some existing SIEM tests, to make sure when a security rule runs, it's generating the new events. Perhaps we can't right now, as that would also make the PR bigger, having to pull in some rule implementations, or a new FT config needs to be created to enable the execlog, or ...

Figured I'd just point this out - we do have some tests like this using some of our FT-only rules (that always fire, or always fail, etc) - let us know if you need some help building tests like that.

++ We'll need to extend SIEM tests, but I wanted to keep that out of the scope of this PR. I think we'll add tests in a separate PR before enabling the eventLog feature flag by default.

kibanamachine · 2021-10-11T11:05:49Z

💚 Build Succeeded

Metrics [docs]

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`securitySolution`	29	30	+1

History

💚 Build #158549 succeeded d00aedffb0739cbf60caac557eaefce3e45a439c
💛 Build #157920 was flaky e6ac12877d58becea022bbdcfdec09c6a6b445d2
💚 Build #157661 succeeded 50ded625252d3eb29019899fdd5dbdc940927f3a
💚 Build #156733 succeeded abf4c8640c0e4a1b2b75eceeeb7fbe6df89933ec
💔 Build #156702 failed 69baec1691c7b29e703712f27fe7667c1ac775c0

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @xcrzx

kibanamachine · 2021-10-11T11:08:56Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

Co-authored-by: Dmitry Shevchenko <[email protected]>

xcrzx force-pushed the event-log-4-exec-log branch 3 times, most recently from 50e175d to bc4fe53 Compare September 16, 2021 11:53

xcrzx changed the title ~~Integrate EventLog to ExecLog~~ [Security Solution] Integrate EventLog to ExecLog Sep 16, 2021

xcrzx force-pushed the event-log-4-exec-log branch from bc4fe53 to 534809c Compare September 16, 2021 15:29

banderror assigned xcrzx Sep 23, 2021

spong mentioned this pull request Sep 24, 2021

[RAC][Alerting][Security Solution] Adds Rule Execution UUID #113058

Merged

2 tasks

xcrzx force-pushed the event-log-4-exec-log branch 5 times, most recently from e4b69ef to 1149b28 Compare September 28, 2021 12:37

xcrzx marked this pull request as ready for review September 28, 2021 13:29

xcrzx requested review from a team as code owners September 28, 2021 13:29

spong approved these changes Sep 29, 2021

View reviewed changes

x-pack/plugins/event_log/generated/mappings.json Outdated Show resolved Hide resolved

...urity_solution/server/lib/detection_engine/rule_execution_log/event_log_adapter/constants.ts Outdated Show resolved Hide resolved

xcrzx force-pushed the event-log-4-exec-log branch 2 times, most recently from 3bda9df to a9484e4 Compare September 29, 2021 10:10

xcrzx force-pushed the event-log-4-exec-log branch from a9484e4 to 8c279c6 Compare September 29, 2021 15:08

banderror reviewed Sep 29, 2021

View reviewed changes

pmuellr requested changes Sep 29, 2021

View reviewed changes

xcrzx force-pushed the event-log-4-exec-log branch from 8c279c6 to 69baec1 Compare September 30, 2021 10:58

banderror changed the title ~~[Security Solution] Integrate EventLog to ExecLog~~ [Security Solution][Detections] Implement writing rule execution events to event_log Sep 30, 2021

banderror added Feature:EventLog Feature:Rule Monitoring Security Solution Detection Rule Monitoring area labels Sep 30, 2021

xcrzx force-pushed the event-log-4-exec-log branch from 69baec1 to abf4c86 Compare September 30, 2021 12:29

ymao1 reviewed Sep 30, 2021

View reviewed changes

...urity_solution/server/lib/detection_engine/rule_execution_log/event_log_adapter/constants.ts Show resolved Hide resolved

x-pack/plugins/event_log/generated/mappings.json Show resolved Hide resolved

x-pack/plugins/event_log/generated/mappings.json Outdated Show resolved Hide resolved

xcrzx force-pushed the event-log-4-exec-log branch 2 times, most recently from 50ded62 to e6ac128 Compare October 5, 2021 09:01

xcrzx requested a review from pmuellr October 5, 2021 12:42

banderror approved these changes Oct 6, 2021

View reviewed changes

xcrzx force-pushed the event-log-4-exec-log branch from e6ac128 to d00aedf Compare October 6, 2021 16:00

pmuellr mentioned this pull request Oct 7, 2021

[event log] add a millisecond version of kibana.task.schedule_delay to the schema #114343

Open

pmuellr approved these changes Oct 7, 2021

View reviewed changes

Integrate EventLog to ExecLog

2bf9fd9

xcrzx force-pushed the event-log-4-exec-log branch from d00aedf to 2bf9fd9 Compare October 11, 2021 09:03

xcrzx enabled auto-merge (squash) October 11, 2021 10:58

xcrzx merged commit ce7b1ea into elastic:master Oct 11, 2021

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 11, 2021

Implement writing rule execution events to event_log (elastic#112286)

71eb45d

kibanamachine mentioned this pull request Oct 11, 2021

[7.x] Implement writing rule execution events to event_log (#112286) #114466

Merged

xcrzx deleted the event-log-4-exec-log branch October 11, 2021 15:27

kibanamachine added a commit that referenced this pull request Oct 11, 2021

Implement writing rule execution events to event_log (#112286) (#114466)

fc35816

Co-authored-by: Dmitry Shevchenko <[email protected]>

banderror mentioned this pull request Nov 11, 2021

[Security Solution] Rule Execution Log - technical debt #118324

Open

19 tasks

spong mentioned this pull request Mar 7, 2022

[Alerting] Track number of queries, query and search time during each rule execution. #125096

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Detections] Implement writing rule execution events to event_log #112286

[Security Solution][Detections] Implement writing rule execution events to event_log #112286

xcrzx commented Sep 15, 2021 •

edited

Loading

elasticmachine commented Sep 28, 2021

elasticmachine commented Sep 28, 2021

spong left a comment

banderror left a comment

pmuellr left a comment

xcrzx commented Oct 5, 2021

banderror left a comment

pmuellr left a comment

pmuellr Oct 7, 2021

xcrzx Oct 11, 2021

pmuellr Oct 7, 2021

xcrzx Oct 11, 2021

kibanamachine commented Oct 11, 2021

kibanamachine commented Oct 11, 2021

[Security Solution][Detections] Implement writing rule execution events to event_log #112286

[Security Solution][Detections] Implement writing rule execution events to event_log #112286

Conversation

xcrzx commented Sep 15, 2021 • edited Loading

Summary

Checklist

elasticmachine commented Sep 28, 2021

elasticmachine commented Sep 28, 2021

spong left a comment

Choose a reason for hiding this comment

banderror left a comment

Choose a reason for hiding this comment

pmuellr left a comment

Choose a reason for hiding this comment

xcrzx commented Oct 5, 2021

banderror left a comment

Choose a reason for hiding this comment

pmuellr left a comment

Choose a reason for hiding this comment

pmuellr Oct 7, 2021

Choose a reason for hiding this comment

xcrzx Oct 11, 2021

Choose a reason for hiding this comment

pmuellr Oct 7, 2021

Choose a reason for hiding this comment

xcrzx Oct 11, 2021

Choose a reason for hiding this comment

kibanamachine commented Oct 11, 2021

💚 Build Succeeded

Metrics [docs]

Public APIs missing exports

History

kibanamachine commented Oct 11, 2021

💚 Backport successful

xcrzx commented Sep 15, 2021 •

edited

Loading